tests/cli/commands/test_cli_secrets.py (1)

464-466: Deduplicate and reuse the same output normalization.

Keep test behavior consistent by calling the same helper here.

Apply this diff:

-        clean_text = " ".join(
-            re.sub(r"[^\x00-\x7F]+", " ", combined_output).split()
-        ).lower()
+        clean_text = normalize_output(combined_output)

src/mcp_agent/app.py (3)

118-129: Comment/data mismatch in declared tool schema.

The comment lists tool_wrapper but the stored dicts don’t include it. Align the comment or add the field.

Apply this diff to fix the comment:

-        #   "tool_wrapper": Callable | None,

---

602-626: Guard against duplicate sync tool declarations.

Repeated decoration with the same name appends duplicates to _declared_tools. Add a simple dedupe guard.

Apply this diff:

         def decorator(fn: Callable[..., Any]) -> Callable[..., Any]:
             tool_name = name or fn.__name__
+            # Idempotency: avoid duplicate declarations
+            if any(d.get("name") == tool_name and d.get("mode") == "sync" for d in self._declared_tools):
+                return fn
             # Construct the workflow from function

---

642-663: Guard against duplicate async tool declarations.

Mirror the sync dedupe to avoid multiple entries for the same async tool.

Apply this diff:

         def decorator(fn: Callable[..., Any]) -> Callable[..., Any]:
             workflow_name = name or fn.__name__
+            if any(d.get("name") == workflow_name and d.get("mode") == "async" for d in self._declared_tools):
+                return fn
             workflow_cls = self._create_workflow_from_function(

tests/cli/commands/test_app_workflows.py (4)

1-2: Fix misleading module docstring

This file tests the “workflows” command, not “configure”.

-"""Tests for the configure command."""
+"""Tests for the app workflows command."""

---

61-74: Use timezone-aware datetimes in test data

Stay consistent with UTC-aware datetimes used elsewhere to avoid tz-related assertions later.

-    app = MCPApp(
+    app = MCPApp(
         appId=MOCK_APP_ID,
         name="name",
         creatorId="creatorId",
-        createdAt=datetime.datetime.now(),
-        updatedAt=datetime.datetime.now(),
+        createdAt=datetime.datetime.now(datetime.timezone.utc),
+        updatedAt=datetime.datetime.now(datetime.timezone.utc),
         appServerInfo=app_server_info,
     )

---

95-106: Same timezone-awareness nit

Mirror the UTC-aware datetime change for AppConfiguration creation to keep consistency.

-    app_config = MCPAppConfiguration(
+    app_config = MCPAppConfiguration(
         appConfigurationId=MOCK_APP_CONFIG_ID,
         creatorId="creator",
         appServerInfo=app_server_info,
     )

(If createdAt/updatedAt fields get added here in future, prefer timezone-aware values.)

Also applies to: 110-113, 121-124

---

61-61: Rename test functions for clarity

Names say “status” but this file tests “workflows”.

-def test_status_app(patched_workflows_app, mock_mcp_client):
+def test_workflows_app(patched_workflows_app, mock_mcp_client):
@@
-def test_status_app_config(patched_workflows_app, mock_mcp_client):
+def test_workflows_app_config(patched_workflows_app, mock_mcp_client):

Also applies to: 95-95

tests/cli/commands/test_app_status.py (3)

1-2: Fix misleading module docstring

This file tests the “status” command.

-"""Tests for the configure command."""
+"""Tests for the app status command."""

---

61-74: Use timezone-aware datetimes in test data

Align with UTC-aware datetimes used by mocks.

-    app = MCPApp(
+    app = MCPApp(
         appId=MOCK_APP_ID,
         name="name",
         creatorId="creatorId",
-        createdAt=datetime.datetime.now(),
-        updatedAt=datetime.datetime.now(),
+        createdAt=datetime.datetime.now(datetime.timezone.utc),
+        updatedAt=datetime.datetime.now(datetime.timezone.utc),
         appServerInfo=app_server_info,
     )

---

95-106: Minor: Ensure tz-aware datetimes if added

If AppConfiguration objects later include timestamps, prefer timezone-aware values for consistency.

Also applies to: 110-113, 121-124

tests/server/test_app_server_workflow_schema.py (1)

24-58: Assert schema by parsing JSON instead of substring search

Parsing the embedded JSON makes the test less brittle to formatting.

-    desc = kwargs.get("description", "")
-    # The description embeds the JSON schema; assert basic fields are referenced
-    assert "q" in desc
-    assert "flag" in desc
-    assert "self" not in desc
+    desc = kwargs.get("description", "")
+    # Extract the first JSON object from the description and parse it
+    import json, re
+    m = re.search(r'\{.*\}', desc, flags=re.S)
+    assert m, "No JSON schema found in description"
+    schema = json.loads(m.group(0))
+    props = schema.get("properties", {})
+    assert "q" in props
+    assert "flag" in props
+    assert "self" not in props

tests/server/test_tool_decorators.py (2)

42-57: Attach app to server_context to avoid fallback heuristics

Make tests more direct by including app on the context used by server helpers.

-    ctx = SimpleNamespace()
+    ctx = SimpleNamespace()
@@
-    req = SimpleNamespace(lifespan_context=server_context)
+    req = SimpleNamespace(lifespan_context=server_context)
     ctx.request_context = req
     ctx.fastmcp = SimpleNamespace(_mcp_agent_app=None)
     return ctx

Additionally, when constructing server_context:

-    server_context = type(
-        "SC", (), {"workflows": app.workflows, "context": app.context}
-    )()
+    server_context = type(
+        "SC", (), {"workflows": app.workflows, "context": app.context, "app": app}
+    )()

---

95-111: Factor polling into a tiny helper to reduce flakiness/noise

DRY the completion wait and centralize timeout.

@@
-    for _ in range(200):
-        status = await _workflow_status(ctx, run_id, "echo")
-        if status.get("completed"):
-            break
-        await asyncio.sleep(0.01)
+    async def _wait_done(name, rid, timeout_s=2.0):
+        for _ in range(int(timeout_s / 0.01)):
+            s = await _workflow_status(ctx, rid, name)
+            if s.get("completed"):
+                return s
+            await asyncio.sleep(0.01)
+        return s
+    status = await _wait_done("echo", run_id)
@@
-    for _ in range(100):
-        status = await _workflow_status(ctx, run_id, "wrapme")
-        if status.get("completed"):
-            break
-        await asyncio.sleep(0.01)
+    status = await _wait_done("wrapme", run_id)

Also applies to: 158-174

src/mcp_agent/server/app_server.py (2)

151-160: Param-source resolution for schema is correct

Good fallback to workflow.run when a custom param-source isn’t present. Consider annotating the return type as Callable[..., Any] for clarity.

---

536-546: Guard against missing/unknown declarations

If a declaration is missing source_fn or has an unexpected mode, we silently continue. Consider logging at debug/warn to aid diagnosis.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: saqadri
PR: lastmile-ai/mcp-agent#386
File: src/mcp_agent/mcp/mcp_server_registry.py:110-116
Timestamp: 2025-08-28T15:07:09.951Z
Learning: In MCP server registry methods, when client_session_factory parameters are updated to accept additional context parameters, ensure the type hints match what is actually passed (Context instance vs ServerSession) and that the default factory (MCPAgentClientSession) can handle the number of arguments being passed to avoid TypeError at runtime.